String dtype: use 'str' string alias and representation for NaN-variant of the dtype #59388
Conversation
| # TODO add more informative repr | ||
| return self.name |
There was a problem hiding this comment.
To discuss in #59342
| # TODO should dtype == "string" work for the NaN variant? | ||
| if other == "string" or other == self.name: # noqa: PLR1714 |
There was a problem hiding this comment.
This other == "string" is added as special case to (for now) keep dtype == "string" working also for the NaN-variant. See the top-post of this PR for some context.
| @@ -224,7 +224,7 @@ def test_apply_categorical(by_row, using_infer_string): | |||
| result = ser.apply(lambda x: "A") | |||
| exp = Series(["A"] * 7, name="XX", index=list("abcdefg")) | |||
| tm.assert_series_equal(result, exp) | |||
| assert result.dtype == object if not using_infer_string else "string[pyarrow_numpy]" | |||
| assert result.dtype == object if not using_infer_string else "str" | |||
There was a problem hiding this comment.
Maybe we should use this as an opportunity to move away from string comparisons and use the pd.StrDtype instead?
There was a problem hiding this comment.
I changed it in some places to an explicit StringDtype(..) creation where we were testing a series of dtypes, but for now kept using the string alias for convenience in other places.
While chatting with Patrick/Brock/Will, we were thinking to for now keep this returning True also for the NaN-variants (as the current PR was doing) |
…nt of the dtype (#59388)
As we discussed for PDEP-14, the idea is that we would keep "string" for the NA-variant of the string dtype (for backwards compatibility), and then to have a distinguishing string representation and alias, use "str" for the default NaN-variant.
This PR updates StringDtype to make that distinction.
One point of discussion that came up while implementing this, what to do with
dtype == "string"?We have quite some places internally in the code where we use this to check for a StringDtype in general (instead of something like
isinstance(dtype, StringDtype)). But should that exclusively return True for the NA-variants of the StringDtype? (to match the string alias/repr)For now I kept a special case to let this return True, but so I could also update all cases in our own code where we currently use this pattern, and change
dtype == "string"to return False for the NaN-variant (dtype == "str"returns True for the NaN variant, and False for the NA variant)xref #54792